Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix initial window size on Wayland #829

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Dec 19, 2024

Fixes #762

  • Moves window sizing code from Application to MainWindow
  • Sets size request from settings before realizing - this reliably sets the initial window dimensions
  • Resets size request to Application minimum after realizing - this allows the user to resize the window subject to hard-coded minimum dimensions.
  • Avoids a theoretically possible dereference of 'null' on new window action.

NOTE:

  1. All windows are created with the size in settings rather than following the current window size as it is simpler. If it is desired that the current window size is followed then an issue should be raised against which a PR can be produced.
  2. The size in settings is always that of the first window on closing regardless of the order of closing. Again, if that needs changing a separate issue/PR should be raised.

Should be tested on OS8/Wayland, OS8/X as well as OS7.1

@jeremypw jeremypw marked this pull request as ready for review December 19, 2024 11:55
@jeremypw jeremypw requested a review from a team December 19, 2024 11:56
@leolost2605
Copy link
Member

leolost2605 commented Dec 19, 2024

I think a better fix would be to use the get_monitor_at_window instead of the primary one which is

  • supported on Wayland
  • usually the primary one anyways on first launch
  • and if not it makes more sense to adjust the size according to the monitor it's actually on and not to the primary one

(Though I have to say I don't know anything about the terminal codebase 😅)

alainm23
alainm23 previously approved these changes Dec 19, 2024
@jeremypw
Copy link
Collaborator Author

@leolost2605 I'll try it. I was assuming it was the get_geometry () that was the problem tho` - might be as well to have an ultimate hard-coded fallback still.

@jeremypw
Copy link
Collaborator Author

The problem with using get_monitor_at_window () and get_primary_monitor () is that it does not work unless the window is realized (which it isn't at the moment). However if you restore the window size after realizing then setting the default dimensions does not work. In Gtk3 you can use resize () but that does not exist in Gtk4 so I do not want to introduce it here. I think therefore we will have to abandon the idea of resizing the terminal window according to the monitor size as a fallback

@jeremypw jeremypw changed the title Apply minimum dimensions on restore Fix initial window size on Wayland Dec 20, 2024
@jeremypw jeremypw dismissed alainm23’s stale review December 20, 2024 19:14

Substantial changes made since review

@jeremypw jeremypw requested a review from alainm23 December 20, 2024 19:15
@jeremypw jeremypw requested a review from vjr February 6, 2025 12:37
@leolost2605
Copy link
Member

The problem with using get_monitor_at_window () and get_primary_monitor () is that it does not work unless the window is realized (which it isn't at the moment). However if you restore the window size after realizing then setting the default dimensions does not work. In Gtk3 you can use resize () but that does not exist in Gtk4 so I do not want to introduce it here. I think therefore we will have to abandon the idea of resizing the terminal window according to the monitor size as a fallback

Where in GTK3 you can do resize (), in GTK4 setting default size after realizing will work so it can be done :)

(I don't have an opinion here just want to say that it's possible :))

@jeremypw
Copy link
Collaborator Author

jeremypw commented Feb 6, 2025

For no obvious reason the zoom overlay appears (blank) on startup in this branch whereas it does not in master - investigating.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Feb 6, 2025

Zoom_overlay problem due to a change in timing/order of events. Now fixed.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Feb 6, 2025

Where in GTK3 you can do resize (), in GTK4 setting default size after realizing will work so it can be done :)

I am only really interested in fixing the Gtk3 version; the Gtk4 branch already has a different method of persisting window size that does not suffer from this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small window on first run under Wayland
3 participants